Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(react): Convert third party auth 'Set password' page to React #18044

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

LZoog
Copy link
Contributor

@LZoog LZoog commented Nov 20, 2024

Because:

  • There are data-sharing problems with this page currently being on Backbone while other pages in the flow are in React, causing a double sign-in for all 'Set password' flows, plus the inability to sign into Sync and maintain CWTS choices for desktop oauth
  • We want to move completely over from Backbone to React

This commit:

  • Creates 'post_verify/third_party_auth/set_password' page in React with container component
  • Sends web channel messages up to Sync after password create success
  • Shares form logic with Signup, includes useSyncEngine hook for DRYness
  • Changes InlineRecoveryKeySetup to check local storage instead of location state, which prevents needing to prop drill as users should always be signed in and these values available in local storage on this page
  • Returns authPW and unwrapBKey from create password in auth-client

closes FXA-6651


TODO: check that this works rebased on Vijay's branch, unit tests for SetPassword + SetPassword container

**I propose creating a follow up issue for moving some tests out from Signup/Signup container and into FormSetupAccount/useSyncEngines and creating stories for FormSetupAccount then, the diff is already large and we do have coverage for these, they're just not in their refactored components. **

@@ -128,7 +128,7 @@ const getReactRouteGroups = (showReactApp, reactRoute) => {
'post_verify/third_party_auth/callback',
'post_verify/third_party_auth/set_password',
]),
fullProdRollout: false,
fullProdRollout: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't roll out until we also turn the feature flag on. I'll double check that's set up properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LZoog LZoog marked this pull request as ready for review November 21, 2024 21:13
@LZoog LZoog requested review from a team as code owners November 21, 2024 21:13
Copy link
Contributor

@bcolsson bcolsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem with existing strings. There is an empty .ftl file being added, not sure if there should be strings there.

Copy link
Contributor

@vpomerleau vpomerleau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good - let's get it up for testing/QA on stage!

Because:
* There are data-sharing problems with this page currently being on Backbone while other pages in the flow are in React, causing a double sign-in for all 'Set password' flows, plus the inability to sign into Sync and maintain CWTS choices for desktop oauth
* We want to move completely over from Backbone to React

This commit:
* Creates 'post_verify/third_party_auth/set_password' page in React with container component
* Sends web channel messages up to Sync after password create success
* Shares form logic with Signup, includes useSyncEngine hook for DRYness
* Changes InlineRecoveryKeySetup to check local storage instead of location state, which prevents needing to prop drill as users should always be signed in and these values available in local storage on this page
* Returns authPW and unwrapBKey from create password in auth-client

closes FXA-6651
@LZoog LZoog merged commit da9ec5a into main Nov 26, 2024
26 checks passed
@LZoog LZoog deleted the FXA-6651 branch November 26, 2024 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants